Add type annotations to the module and the unit tests#6
Conversation
|
@ilanbiala: with this, it should be possible to implement full type hinting support for the DBC submodule of cantools... |
|
updated python version tested in CI to 3.10 and 3.14. (python 3.8 has been EOL for a while.) |
andlaus
force-pushed
the
add_type_annotations
branch
from
c3d76d3 to
2ddd392
Compare
|
fun fact: github "simplifies" the |
|
It’s because 3.10 is a number, both in yaml and json. It’s a bit silly that GitHub accepts anything but strings :) |
|
@andlaus is it possible to split out the Python version bump changes into a separate PR and merge that first to isolate this work? |
|
Hey @eerimoq! Sorry to bring this up here, but would it be possible for you to give @andlaus, @zariiii9003, myself, and others more access to the cantools repo and for the readthedocs site for Cantools as well (cantools/cantools#746)? |
yes, but the version bump needs to be done before adding the type hints. be aware that the minimum python version is currently not codified in |
what exactly? |
andlaus
force-pushed
the
add_type_annotations
branch
from
2ddd392 to
a45598c
Compare
|
@eerimoq the Cantools readthedocs site is stuck on a commit that's 2.5 years old and a few version releases out of date (commit revision is at the bottom of the docs site). It looks like permissions haven't been set up correctly for other people to push to the docs site. |
For some of these I am not 100% sure if they are correct. A lot of the code churn of this patch stems from the fact that some classes had to be reordered because python bails out if annotations use classes that are only defined later in the file. This patch features a few minor functional changes which were neccessiated by type annotations: - Support for python2 is no longer declared in `setup.py` - The `Parser.grammar()` methods are supposed to return `Grammar` objects instead of a `Pattern` object which gets implicitly converted to `Grammar` in `Parser.parse()`. IMO this makes the whole affair much less confusing and user code only needs to return `Grammar(x)` instead of `x`. If `grammar()` returns something else, the old behavior is fallen back to, but this will lead to `mypy` complaints in user code... - The `_Token` class is converted from `namedtuple` to a dataclass because named tuples do not play nicely with type annotations and the performance is basically identical. - The internal `_String` class now uses `Pattern` as its base class. (It already had the same API as `Pattern`, but not deriving `_String` from Pattern would necessiate `_String` to be handled separately anyway.) Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com> Approved-by: Christian Hackenbeck <christian.hackenbeck@mercedes-benz.com>
andlaus
force-pushed
the
add_type_annotations
branch
from
a45598c to
79df0af
Compare
|
ok, I rebased this on top of the latest master version, i.e., no need to fiddle with the python versions anymore... |
|
@andlaus could you update the gha config to run mypy in CI? |
|
good idea. I also added a ruff linting step. |
thanks to [at]zariiii9003 for the suggestion! Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
also fix the complaint about the unused import of the `typing` module. Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
andlaus
force-pushed
the
add_type_annotations
branch
from
724b63c to
64b4204
Compare
4 participants
For some of these I am not 100% sure if they are correct.
A lot of the code churn of this patch stems from the fact that some classes had to be reordered because python bails out if annotations use classes that are only defined later in the file.
This patch features a few minor functional changes which were neccessiated by type annotations:
setup.pyParser.grammar()methods always returnGrammarobjects instead of aPatternobject which gets implicitly converted toGrammarinParser.parse(). (IMO this makes the whole affair much less confusing and user code only needs to returnGrammar(x)instead ofx.)_Tokenclass is converted fromnamedtupleto a dataclass because named tuples do not play nicely with type annotations and the performance is basically identical._Stringclass now usesPatternas its base class. (It already had the same API asPattern, but not deriving_Stringfrom Pattern would necessiate_Stringto be handled separately anyway.)